[scautoloc] Export secondaryAzimuthalGap in OriginQuality#116
Conversation
comoglu
force-pushed
the
fix/scautoloc-secondary-azimuthal-gap
branch
from
9130cc4 to
1c32d36
Compare
|
Now the azimuthal gap will be computed two times in |
|
This is the exact idea, to get rid of redundant code by using the |
|
So this PR is a draft and not about to be merged? |
|
One thing I'd like to understand better: in the autoloc context, are DataModel::Pick objects registered in the global public object store at the point convertToSC() is called? compile() relies on Pick::Find() for station counts, and I noticed the Pick::Find() block in convertFromSC() was commented out with a note (german) . Wondering if that affects the station count fields if we go the compile() route. |
|
I converted the PR to Draft until I understand that part better. |
|
Following up on the compile() question I raised in April: I tested whether DataModel::Pick objects are still registered in the global PublicObject store at the point convertToSC() is called in the autoloc context. They are — picks arrive via messaging moments before the origin is processed and SC hasn't cleaned them up yet, so Pick::Find() would succeed there. That resolves my concern about the compile() route for station counts. @jsaul — given that, would you prefer this PR moves toward using compile() to consolidate the redundant azimuthal gap computation (as you suggested), or is the current approach fine to merge as-is and the consolidation handled separately as part of cleanup? |
scautoloc computes the secondary azimuthal gap internally via determineAzimuthalGaps() and uses it as a rejection criterion, but the value was not written into OriginQuality when exporting the origin to SeisComP. This caused scautoloc origins to always have secondaryAzimuthalGap unset, unlike stdloc and scrtdd which both populate it. Call determineAzimuthalGaps() in convertToSC() and set secondaryAzimuthalGap on the exported origin.
comoglu
force-pushed
the
fix/scautoloc-secondary-azimuthal-gap
branch
from
1c32d36 to
218f008
Compare
|
Rebased onto the current main, conflict resolved, and the PR is now mergeable again. @jsaul @gempa-jabe if you have a moment, this small fix has been open for a while. |
|
I am fine with that change. Does it work as intended? Has it been tested? |
|
Thanks @gempa-jabe
I have just tested it again with a playback. Before secondaryAzimuthalGap
was unset, after it is populated with the correct values.So it works as
intended.
…On Thu, Jul 16, 2026 at 9:45 PM Jan Becker ***@***.***> wrote:
*gempa-jabe* left a comment (SeisComP/main#116)
<#116 (comment)>
I am fine with that change. Does it work as intended? Has it been tested?
—
Reply to this email directly, view it on GitHub
<#116?email_source=notifications&email_token=ACB5ZSEH7ASIACJIAEKVXXL5FC56DA5CNFSNUABFM5UWIORPF5TWS5BNNB2WEL2JONZXKZKDN5WW2ZLOOQXTIOJZGE2DGMRUG4ZKM4TFMFZW63VGMF2XI2DPOKSWK5TFNZ2KYZTPN52GK4S7MNWGSY3L#issuecomment-4991432472>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ACB5ZSAYHI5A6TDQ5VIN77D5FC56DAVCNFSNUABFKJSXA33TNF2G64TZHMZDKMBSGIZTGNJYHNEXG43VMU5TIMRXHA4TANZXGU3KC5QC>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
|
I believe that it will do what this PR is about. I was just curios if it does not brake anything. So the remaining attributes should be the same as before. If @jsaul is fine with it, I can merge it. |
|
I checked it. The primary azimuthalGap doesn't change. determineAzimuthalGaps() computes it the same way geoProperties() did, using the same inputs (the max consecutive gap over non-excluded arrival azimuths with a 360° wrap). If there are fewer than 2 arrivals it just falls back to the old geoProperties value. Nothing else in OriginQuality is touched, the only new thing is secondaryAzimuthalGap. The playback confirmed it as well: the primary gap values are the same as before and the secondary is always greater than or equal to the primary, as expected. |
|
Fine with it. 👍 |
3 participants
Problem
scautoloc computes the secondary azimuthal gap internally via
determineAzimuthalGaps()and uses it as a rejection criterion, but the value was never written intoOriginQualitywhen exporting the origin to SeisComP. As a result, scautoloc origins always havesecondaryAzimuthalGapunset — unlike stdloc and scrtdd which both populate it.This was confirmed as an oversight by @jsaul:
Fix
Call
determineAzimuthalGaps()inconvertToSC()(sc3adapters.cpp) and setsecondaryAzimuthalGapon the exportedOriginQuality. The function was already available and correct — it just wasn't called at export time.Related
Companion PR in SeisComP/common adds a standalone
computeAzimuthalGaps(azimuths, primary*, secondary*)library function so the algorithm has a single home, as @jsaul suggested: SeisComP/common#193